<html>
<head><meta charset="utf-8"><title>arena · t-cargo/PubGrub · Zulip Chat Archive</title></head>
<h2>Stream: <a href="https://rust-lang.github.io/zulip_archive/stream/260232-t-cargo/PubGrub/index.html">t-cargo/PubGrub</a></h2>
<h3>Topic: <a href="https://rust-lang.github.io/zulip_archive/stream/260232-t-cargo/PubGrub/topic/arena.html">arena</a></h3>

<hr>

<base href="https://rust-lang.zulipchat.com">

<head><link href="https://rust-lang.github.io/zulip_archive/style.css" rel="stylesheet"></head>

<a name="227962531"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/260232-t-cargo/PubGrub/topic/arena/near/227962531" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Matthieu Pizenberg <a href="https://rust-lang.github.io/zulip_archive/stream/260232-t-cargo/PubGrub/topic/arena.html#227962531">(Feb 26 2021 at 17:37)</a>:</h4>
<p><span class="user-mention" data-user-id="120179">@Eh2406</span> in the arena module why are you defining the clone, copy, eq, ... traits instead of deriving them? is this because of the phantom data? By the way it's the first time I see use of it so I'm discovering it a bit while reading the code</p>



<a name="227962863"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/260232-t-cargo/PubGrub/topic/arena/near/227962863" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Eh2406 <a href="https://rust-lang.github.io/zulip_archive/stream/260232-t-cargo/PubGrub/topic/arena.html#227962863">(Feb 26 2021 at 17:40)</a>:</h4>
<p>IDK, try removing the impls and adding the derive. If it works, and the test pass, then I did it by accident.</p>



<a name="227963283"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/260232-t-cargo/PubGrub/topic/arena/near/227963283" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Eh2406 <a href="https://rust-lang.github.io/zulip_archive/stream/260232-t-cargo/PubGrub/topic/arena.html#227963283">(Feb 26 2021 at 17:42)</a>:</h4>
<blockquote>
<p>phantom data</p>
</blockquote>
<p>is just a way to tell the type system that I am using a T, while telling the code generator that I am not.<br>
This lets us keep track of the fact that this u32 does not have the same type as one from a different arena. But at runtime it is just a u32.</p>



<a name="227964083"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/260232-t-cargo/PubGrub/topic/arena/near/227964083" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Matthieu Pizenberg <a href="https://rust-lang.github.io/zulip_archive/stream/260232-t-cargo/PubGrub/topic/arena.html#227964083">(Feb 26 2021 at 17:47)</a>:</h4>
<p><span class="user-mention silent" data-user-id="120179">Eh2406</span> <a href="#narrow/stream/260232-t-cargo.2FPubGrub/topic/arena/near/227962863">said</a>:</p>
<blockquote>
<p>IDK, try removing the impls and adding the derive. If it works, and the test pass, then I did it by accident.</p>
</blockquote>
<p>Indeed, deriving some properties requires additional bounds on <code>T</code> that we actually do not need because it's phantom, so that's why you manually defined those. Some people link to this thread regarding similar cases with derive: <a href="https://github.com/rust-lang/rust/issues/26925">https://github.com/rust-lang/rust/issues/26925</a></p>



<a name="227965360"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/260232-t-cargo/PubGrub/topic/arena/near/227965360" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Eh2406 <a href="https://rust-lang.github.io/zulip_archive/stream/260232-t-cargo/PubGrub/topic/arena.html#227965360">(Feb 26 2021 at 17:54)</a>:</h4>
<p>That makes sense. Can you add a comment explaining that?</p>



<a name="227965391"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/260232-t-cargo/PubGrub/topic/arena/near/227965391" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Eh2406 <a href="https://rust-lang.github.io/zulip_archive/stream/260232-t-cargo/PubGrub/topic/arena.html#227965391">(Feb 26 2021 at 17:54)</a>:</h4>
<p>Or do you want me to?</p>



<a name="227965406"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/260232-t-cargo/PubGrub/topic/arena/near/227965406" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Matthieu Pizenberg <a href="https://rust-lang.github.io/zulip_archive/stream/260232-t-cargo/PubGrub/topic/arena.html#227965406">(Feb 26 2021 at 17:55)</a>:</h4>
<p>Yep just did that</p>



<a name="227965461"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/260232-t-cargo/PubGrub/topic/arena/near/227965461" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Matthieu Pizenberg <a href="https://rust-lang.github.io/zulip_archive/stream/260232-t-cargo/PubGrub/topic/arena.html#227965461">(Feb 26 2021 at 17:55)</a>:</h4>
<p>I'll do a few commits for docs of things I come accross while reading</p>



<a name="227970481"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/260232-t-cargo/PubGrub/topic/arena/near/227970481" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Matthieu Pizenberg <a href="https://rust-lang.github.io/zulip_archive/stream/260232-t-cargo/PubGrub/topic/arena.html#227970481">(Feb 26 2021 at 18:28)</a>:</h4>
<p>I'll write few things down here while reading. Don't feel like you need to answer those now. It's just so that you know what things are going through my head while reading. And I might even find answers to some of those on my own while continuing reading. Just doing that so that I don't post a ton of things all at once. But I'll keep those notes and organize a summary of my questions at the end.</p>



<a name="227970499"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/260232-t-cargo/PubGrub/topic/arena/near/227970499" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Matthieu Pizenberg <a href="https://rust-lang.github.io/zulip_archive/stream/260232-t-cargo/PubGrub/topic/arena.html#227970499">(Feb 26 2021 at 18:28)</a>:</h4>
<p>[ ] std::any::type_name wasn't that the one with a security issue at some point?<br>
[ ] Should we start bigger for Arena::new()?<br>
[ ] why using id.into_raw() for Derived.shared_id? It is used in the reporting code, but we actually do not need that to be a usize per-se. So going forward, we should refactor the reporting code to use the <code>Id</code> type instead and not having to expose an <code>into_raw</code> method.</p>



<a name="227980384"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/260232-t-cargo/PubGrub/topic/arena/near/227980384" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Matthieu Pizenberg <a href="https://rust-lang.github.io/zulip_archive/stream/260232-t-cargo/PubGrub/topic/arena.html#227980384">(Feb 26 2021 at 19:36)</a>:</h4>
<p>[ ] do we actually need Incompatibility::from_dependencies? It is allocating a vec, and it seems it's only used to do two things, (1) iterate over newly created incompats in solver::resolve and add them to the incompatibility store (arena), and (2) add the version to the partial solution if the dependencies are not problematic. (2) needs references to those incompatibilities, which could be done via the Arena if it implemented something like slices or iterators starting at a given Id and of a given size. (don't know if that's possible)</p>



<a name="227986859"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/260232-t-cargo/PubGrub/topic/arena/near/227986859" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Eh2406 <a href="https://rust-lang.github.io/zulip_archive/stream/260232-t-cargo/PubGrub/topic/arena.html#227986859">(Feb 26 2021 at 20:18)</a>:</h4>
<blockquote>
<p>std::any::type_name wasn't that the one with a security issue at some point?</p>
</blockquote>
<p>I don't think so, but now I can't find it. I think the one with the issues let you take an ID and turn it into a T.<br>
<a href="https://blog.rust-lang.org/2019/05/13/Security-advisory.html">https://blog.rust-lang.org/2019/05/13/Security-advisory.html</a></p>



<a name="227987585"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/260232-t-cargo/PubGrub/topic/arena/near/227987585" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Eh2406 <a href="https://rust-lang.github.io/zulip_archive/stream/260232-t-cargo/PubGrub/topic/arena.html#227987585">(Feb 26 2021 at 20:23)</a>:</h4>
<blockquote>
<p>Should we start bigger for Arena::new()?</p>
</blockquote>
<p>worth experimenting! Not worth holding things up for.</p>



<a name="227987994"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/260232-t-cargo/PubGrub/topic/arena/near/227987994" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Eh2406 <a href="https://rust-lang.github.io/zulip_archive/stream/260232-t-cargo/PubGrub/topic/arena.html#227987994">(Feb 26 2021 at 20:26)</a>:</h4>
<blockquote>
<p>So going forward, we should refactor the reporting code to use the Id type instead and not having to expose an into_raw method.</p>
</blockquote>
<p>Yes! But that is a braking change and so far we have not made any on dev. So let's look into that after a 2.1</p>



<a name="227988363"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/260232-t-cargo/PubGrub/topic/arena/near/227988363" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Eh2406 <a href="https://rust-lang.github.io/zulip_archive/stream/260232-t-cargo/PubGrub/topic/arena.html#227988363">(Feb 26 2021 at 20:29)</a>:</h4>
<blockquote>
<p>do we actually need Incompatibility::from_dependencies</p>
</blockquote>
<p>Did not think of this! We should definitely experiment!</p>



<a name="228067724"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/260232-t-cargo/PubGrub/topic/arena/near/228067724" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Matthieu Pizenberg <a href="https://rust-lang.github.io/zulip_archive/stream/260232-t-cargo/PubGrub/topic/arena.html#228067724">(Feb 27 2021 at 16:24)</a>:</h4>
<p><span class="user-mention silent" data-user-id="120179">Eh2406</span> <a href="#narrow/stream/260232-t-cargo.2FPubGrub/topic/arena/near/227987994">said</a>:</p>
<blockquote>
<blockquote>
<p>So going forward, we should refactor the reporting code to use the Id type instead and not having to expose an into_raw method.</p>
</blockquote>
<p>Yes! But that is a braking change and so far we have not made any on dev. So let's look into that after a 2.1</p>
</blockquote>
<p>Very true!</p>



<a name="228068332"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/260232-t-cargo/PubGrub/topic/arena/near/228068332" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Matthieu Pizenberg <a href="https://rust-lang.github.io/zulip_archive/stream/260232-t-cargo/PubGrub/topic/arena.html#228068332">(Feb 27 2021 at 16:35)</a>:</h4>
<p>[ ] merge_into(id: Id&lt;Self&gt;, incompats: &amp;mut Vec&lt;Id&lt;Self&gt;&gt;) method becomes an associated function<br>
[ ] build_derivation_tree method also becomes an associated function, are they both still located in the right module?</p>



<a name="228068335"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/260232-t-cargo/PubGrub/topic/arena/near/228068335" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Matthieu Pizenberg <a href="https://rust-lang.github.io/zulip_archive/stream/260232-t-cargo/PubGrub/topic/arena.html#228068335">(Feb 27 2021 at 16:35)</a>:</h4>
<p>[ ] I like the simpler State.add_incompatibility!</p>



<a name="228070476"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/260232-t-cargo/PubGrub/topic/arena/near/228070476" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Eh2406 <a href="https://rust-lang.github.io/zulip_archive/stream/260232-t-cargo/PubGrub/topic/arena.html#228070476">(Feb 27 2021 at 17:12)</a>:</h4>
<blockquote>
<p>merge_into</p>
</blockquote>
<p>Should just be inlined. Except then what to do with the comment.<br>
So until we have a plan for how to impl that comment, I don't think there will be a "good place" for it.</p>



<a name="228070519"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/260232-t-cargo/PubGrub/topic/arena/near/228070519" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Matthieu Pizenberg <a href="https://rust-lang.github.io/zulip_archive/stream/260232-t-cargo/PubGrub/topic/arena.html#228070519">(Feb 27 2021 at 17:13)</a>:</h4>
<p>[ ] does conflict_resolution need the id? or can pass the &amp;Incompat?</p>
<div class="codehilite"><pre><span></span><code>    -&gt; it returns the id in the end, is it needed?
    -&gt; it calls build_derivation_tree(id) (same question, need id?)
       -&gt; it calls Incompatibility::build_derivation_tree which needs it since it has to know if it&#39;s a shared id
       BTW seems like build_derivation_tree would be better located directly in the core module.
    -&gt; it calls self.backtrack(id) (same question, need id?)
        -&gt; it calls Incompat::merge_into, which needs the id since it merges into a Vec of Ids
    -&gt; it calls Incompat::prior_cause, which I guess def need the id
</code></pre></div>
<p>[ ] same question for self.partial_solution.add_derivation?</p>



<a name="228087452"></a>
<h4><a href="https://rust-lang.zulipchat.com#narrow/stream/260232-t-cargo/PubGrub/topic/arena/near/228087452" class="zl"><img src="https://rust-lang.github.io/zulip_archive/assets/img/zulip.svg" alt="view this post on Zulip" style="width:20px;height:20px;"></a> Matthieu Pizenberg <a href="https://rust-lang.github.io/zulip_archive/stream/260232-t-cargo/PubGrub/topic/arena.html#228087452">(Feb 27 2021 at 22:34)</a>:</h4>
<p><span class="user-mention" data-user-id="120179">@Eh2406</span> with all the performance work that (mostly you) we have done since 2.0 there is a huge boost. I just relaunched the code solving dependencies of all versions of all elm packages (same from last time, I didn't updated it with the new packages since then). It went from 900ms to 400ms in the latest version</p>



<hr><p>Last updated: Aug 07 2021 at 22:04 UTC</p>
</html>